Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: add parser message template #11192

Merged
merged 10 commits into from Mar 3, 2020

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Mar 1, 2020

Q                       A
Tests Added + Pass? Yes
License MIT

This PR collects the scattered babel parser error messages into a centralized message store. By doing so we can implement expression scops which records different types of errors when parsing ambiguous patterns.

The naming conventions of error messages strictly follows v8's message template if they exists, otherwise I coin some phrase so PTAL on the message template. 🙏

Todo

  • flow plugin
  • typescript plugin
  • estree plugin
  • jsx plugin

@JLHwung JLHwung added PR: Internal 🏠 A type of pull request used for our changelog categories pkg: parser labels Mar 1, 2020
// $FlowIgnore
const err: SyntaxError & { pos: number, loc: Position } = new SyntaxError(
const message =
errorTemplate.replace(/%(\d+)/g, (_, i: number) => params[i]) +
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message now supports naive %0-style templates.

@JLHwung
Copy link
Contributor Author

JLHwung commented Mar 2, 2020

I am not sure if I should do this for TypeScript plugin because the way ts compiler indexes diagnostics is quite brutal: https://github.com/microsoft/TypeScript/blob/9c71eaf59040ae75343da8cdff01344020f5bba2/lib/diagnosticMessages.generated.json

@nicolo-ribaudo
Copy link
Member

We should probably diverge from their error codes, and be consistent with the other parts of our parser.

@JLHwung JLHwung force-pushed the refactor-parser-message-template branch from 3321481 to 5ededb5 Compare March 3, 2020 02:14
@JLHwung JLHwung marked this pull request as ready for review March 3, 2020 02:31
@existentialism
Copy link
Member

Time for a lint rule that prevents this.raise to be called without a constant 😝

Copy link
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the super minor question about consistency, nice work LGTM!

Copy link
Member

@hzoo hzoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

truly superb! goes a long way for maintenance

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

packages/babel-parser/src/parser/location.js Outdated Show resolved Hide resolved
Co-Authored-By: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
@JLHwung JLHwung force-pushed the refactor-parser-message-template branch from 63bd85c to 1af4064 Compare March 3, 2020 17:20
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 3, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants